Provide an easy way to edit the googletest arguments in integration tests #425
Conversation
…ine arguments in integration tests
dconeybe
added
the
tests-requested: quick
github-actions
bot
added
tests: in-progress
firebase-workflow-trigger
bot
removed
the
tests: in-progress
github-actions
bot
added
tests: in-progress
firebase-workflow-trigger
bot
removed
the
tests: in-progress
dconeybe
requested review from
ehsannas and
jonsimantov
and removed request for
jonsimantov
There was a problem hiding this comment.
This is generally useful. I used this method when I was trying to reproduce an issue for a specific test. A minor suggestion inlined.
Also a couple of questions for my own benefit:
Are the changes clang-formatted? (If not, does GitHub automatically format it before merging?)
How is this firebase_test_framework invoked? The cleaner approach seems to be to pass --gtest_filter to that invocation (so that it's already in argv when we get to common_main)
|
|
||
| // Write the elements of the vector back into argv and modify argc. | ||
| // The memory leaks produced below are acceptable because they would last the | ||
| // entire lifetime of the application anyways. |
There was a problem hiding this comment.
More importantly, I would say they are acceptable because this may only be used during development / debugging.
We could alter the code slightly so that it doesn't leak memory except for when someone is adding new flags during dev/debugging process. Meaning, we could add an early exit here:
if (args_vector.size() == argc) {
return argv;
}There was a problem hiding this comment.
Good idea. I've implemented this suggestion and refactored the code a bit. PTAL.
Yes, the file has been formatted with
It would definitely be cleaner to just specify arguments on the command line. You can indeed do this when running the desktop tests. However, Android has no means of specifying command-line arguments and we have not plumbed through the command-line arguments in iOS (if it's even possible). There are probably some platform-specific mechanisms that could be used to specify the arguments without having to edit source code; however, this is just a quick-and-dirty way to get it done. |
|
Got it. Thanks |
… changes to the args were made
dconeybe
added
the
tests-requested: quick
github-actions
bot
added
tests: in-progress
✅ Integration test succeeded!Requested by @dconeybe on commit 8165fad |
github-actions
bot
added
the
tests: succeeded
firebase-workflow-trigger
bot
removed
the
tests: in-progress
2 participants
Android and iOS do not have a way to take command-line arguments as input, and therefore there is no way to specify useful arguments like
--gtest_test_filterwhen developing/debugging a specific test. This PR modifiesfirebase_test_framework.ccto at least provide an easy way to hardcode googletest arguments during development.